- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15k
[AMDGPU] Improve StructurizeCFG pass performance by using SSAUpdaterBulk. #150937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AMDGPU] Improve StructurizeCFG pass performance by using SSAUpdaterBulk. #150937
Conversation
| 
 This stack of pull requests is managed by Graphite. Learn more about stacking. | 
| @llvm/pr-subscribers-llvm-transforms Author: Valery Pykhtin (vpykhtin) ChangesThis is a replacement PR for #130611 and PR llvm/llvm-project #135181 due to the parent branches change. No changes since. Full diff: https://github.com/llvm/llvm-project/pull/150937.diff 1 Files Affected: 
 diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
index 44e63a0583d1a..5d4f2d1a38407 100644
--- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
+++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
@@ -48,6 +48,7 @@
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/SSAUpdater.h"
+#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
 #include <cassert>
 #include <utility>
 
@@ -319,7 +320,7 @@ class StructurizeCFG {
 
   void collectInfos();
 
-  void insertConditions(bool Loops);
+  void insertConditions(bool Loops, SSAUpdaterBulk &PhiInserter);
 
   void simplifyConditions();
 
@@ -668,10 +669,9 @@ void StructurizeCFG::collectInfos() {
 }
 
 /// Insert the missing branch conditions
-void StructurizeCFG::insertConditions(bool Loops) {
+void StructurizeCFG::insertConditions(bool Loops, SSAUpdaterBulk &PhiInserter) {
   BranchVector &Conds = Loops ? LoopConds : Conditions;
   Value *Default = Loops ? BoolTrue : BoolFalse;
-  SSAUpdater PhiInserter;
 
   for (BranchInst *Term : Conds) {
     assert(Term->isConditional());
@@ -680,8 +680,9 @@ void StructurizeCFG::insertConditions(bool Loops) {
     BasicBlock *SuccTrue = Term->getSuccessor(0);
     BasicBlock *SuccFalse = Term->getSuccessor(1);
 
-    PhiInserter.Initialize(Boolean, "");
-    PhiInserter.AddAvailableValue(Loops ? SuccFalse : Parent, Default);
+    unsigned Variable = PhiInserter.AddVariable("", Boolean);
+    PhiInserter.AddAvailableValue(Variable, Loops ? SuccFalse : Parent,
+                                  Default);
 
     BBPredicates &Preds = Loops ? LoopPreds[SuccFalse] : Predicates[SuccTrue];
 
@@ -694,7 +695,7 @@ void StructurizeCFG::insertConditions(bool Loops) {
         ParentInfo = PI;
         break;
       }
-      PhiInserter.AddAvailableValue(BB, PI.Pred);
+      PhiInserter.AddAvailableValue(Variable, BB, PI.Pred);
       Dominator.addAndRememberBlock(BB);
     }
 
@@ -703,9 +704,9 @@ void StructurizeCFG::insertConditions(bool Loops) {
       CondBranchWeights::setMetadata(*Term, ParentInfo.Weights);
     } else {
       if (!Dominator.resultIsRememberedBlock())
-        PhiInserter.AddAvailableValue(Dominator.result(), Default);
+        PhiInserter.AddAvailableValue(Variable, Dominator.result(), Default);
 
-      Term->setCondition(PhiInserter.GetValueInMiddleOfBlock(Parent));
+      PhiInserter.AddUse(Variable, &Term->getOperandUse(0));
     }
   }
 }
@@ -1410,8 +1411,12 @@ bool StructurizeCFG::run(Region *R, DominatorTree *DT,
   orderNodes();
   collectInfos();
   createFlow();
-  insertConditions(false);
-  insertConditions(true);
+
+  SSAUpdaterBulk PhiInserter;
+  insertConditions(false, PhiInserter);
+  insertConditions(true, PhiInserter);
+  PhiInserter.RewriteAndOptimizeAllUses(*DT);
+
   setPhiValues();
   simplifyHoistedPhis();
   simplifyConditions();
 | 
| Can you request an internal psdb full cycle to test this PR? StructurizeCFG seems very sensitive to changes and has caused a lot of downstream failures. | 
| 
 I did that before and it passed, but I'll double check, thanks! | 
05eee4a    to
    4bcba8d      
    Compare
  
    9e8736c    to
    79e1b41      
    Compare
  
    4bcba8d    to
    8433c98      
    Compare
  
    79e1b41    to
    444e960      
    Compare
  
    444e960    to
    6722e83      
    Compare
  
    8433c98    to
    654a23b      
    Compare
  
    6722e83    to
    b5043f6      
    Compare
  
    654a23b    to
    ae3589e      
    Compare
  
    ae3589e    to
    fa7a951      
    Compare
  
    b5043f6    to
    df6dbd1      
    Compare
  
    fa7a951    to
    a7d1d3b      
    Compare
  
    …ulk. (llvm#150937) SSAUpdaterBulk replaces legacy SSAUpdater.

This is a replacement PR for #130611 and PR llvm/llvm-project #135181 due to the parent branches change. No changes since.